Remove reliance on hardcoded user in ci #1170
Conversation
.github/workflows/ci.yml
Outdated
|
|
||
| jobs: | ||
| linux-compat: | ||
| runs-on: ubuntu-20.04 # latest | ||
| runs-on: ubuntu-22.04 # latest |
There was a problem hiding this comment.
some of the older compilers are not available on 24 (clang 3 and gcc 4.8 if i remember correctly). dropping those from our ci is a bridge we can cross later
There was a problem hiding this comment.
BTW, Does this version of Ubuntu matter since our Docker base image is Ubuntu 18?
There was a problem hiding this comment.
actually you are right. it should not affect the compiler cause underlying image is old. let me retest. i saw old compilers failing when i moved to 24, so i assumed its related, but maybe it is not.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1170 +/- ##
=======================================
Coverage 83.63% 83.63%
=======================================
Files 57 57
Lines 5953 5953
=======================================
Hits 4979 4979
Misses 974 974 ☔ View full report in Codecov by Sentry. |
**Issue** A recent PR #1170 (comment) fixed an indexing-math bug in this test. When I took a closer look at this test, I noticed it was broken ... and didn't make much sense **Research:** This hash table test was added with the original hash table PR #17. That branch had 2 different authors, which seems to have led to the brokenness - Original [commit](c2a7f08) with this test - In this [commit](ab4e987), author B accidentally replaces floating-point-rand with integer-rand, breaking randomness so this test doesn't actually test anything anymore - In this [commit](fe71f05), Author A removes some `entries[i -1]` checks that don't really make sense because `entries` isn't sorted - In this [commit](a36d2dd), Author B brings the checks back. My guess is there was a merge conflict and they just accepted their own side **Description of changes** - Fix randomness - Add comments about what this test is doing - Fix the checks, using the `sorted_entries` array instead of the unsorted `entries` array
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.